in noImplicitReturns mode, also disallow "return;" #7358
Conversation
|
Hi @martine, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution! The agreement was validated by Microsoft and real humans are currently evaluating your PR. TTYL, MSBOT; |
|
The Travis build appears to have failed due to a lint error (?). I don't see any mention of lint in |
| @@ -13880,6 +13880,11 @@ namespace ts { | |||
| checkTypeAssignableTo(exprType, returnType, node.expression); | |||
| } | |||
| } | |||
| } else { | |||
There was a problem hiding this comment.
Else on the next line (we link to a style guide in CONTRIBUTING.md)
|
Can you rename the test file? It's not that there's an empty statement, it's that the return statement lacks a return expression. Other than that and a few style nits, this looks good. @vladima, can you also take a look? |
|
Also, |
|
So embarrassing that I got tabs wrong, it's such a pet peeve of mine when others get it wrong! I think I have fixed the issues you brought up. I'm still not quite sure I'm checking for the right thing (e.g. isTypeOfKind vs maybeTypeOfKind) but I added some more test cases to verify it makes some sense. |
| @@ -13881,6 +13881,10 @@ namespace ts { | |||
| } | |||
| } | |||
| } | |||
| else if (compilerOptions.noImplicitReturns && !isTypeOfKind(returnType, TypeFlags.Void)) { | |||
There was a problem hiding this comment.
void | any. similar to other places where we check noImpicitReturns.
There was a problem hiding this comment.
With your suggestion, this code is now accepted. Is that expected? (I don't have a good intuition about how "any" ought to behave, so I just wanted to double check.)
function isMissingReturnExpression2(): any {
return;
}
I have made the change regardless.
|
also can you add a test for an implicit return type that is inferred from the function body: function f(x) {
if (x) {
return 0;
}
else {
return;
}
} |
In --noImplicitReturns mode, if a function specifies a return type, disallow empty "return;" statements. Fixes #5916.
|
Done. I also fixed the tabs in my test code (adding the "if" statement made me realize I had it wrong). |
|
👍 |
|
Wow.. @traviceCI is taking its time for this commit.. |
|
The suspense... |
in noImplicitReturns mode, also disallow "return;"
|
thanks @martine! |
In --noImplicitReturns mode, if a function specifies a return type,
disallow empty "return;" statements.
Fixes #5916.
This is my first TypeScript change, so please review with caution.
Some things wasn't sure about, checked off the resolved ones:
voidreturn etc.